-
Notifications
You must be signed in to change notification settings - Fork 4.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Don't allow empty chat messages #1137
Conversation
@kevinmitch14 is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Added a further enhancement to disable autocomplete on the chat input. Spotted it on the preview deployment! |
]) | ||
|
||
event.currentTarget.message.value = "" | ||
if (messageLength > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using a State to maintain and check for message length, try using "event.currentTarget.message.value" directly to check message length. something like this:-
const messageInput = event.currentTarget.message.value;
if(messageInput?.length > 0){
// Add Message
}
/// or just return from function if message is empty
if(messageInput?.length < 1){
return
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @SRX9, I am using state to track the message length in order to disable/enable the send button on the form which is seen in most chat apps today.
Using event.currentTarget.message.value
works also, it does mean that the messageInput type is any
. And then you continue to use trim()
and .length
on something of type any which shows no problems on the surface. Changing the id of the input will also easily go unnoticed.
Both options work, but the state would still be there to disable/enable send button when input length is < 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Hi @kevinmitch14, thanks for the PR. Can you please simplify this by using a ref like this: export function CardsChat() {
const [open, setOpen] = React.useState(false)
const [selectedUsers, setSelectedUsers] = React.useState<User[]>([])
const inputRef = React.useRef<HTMLInputElement>(null) <CardFooter>
<form
onSubmit={(event) => {
event.preventDefault()
if (!event.currentTarget.message.value) return <Input
ref={inputRef}
id="message"
placeholder="Type your message..."
className="flex-1"
/> |
Hi @dan5py, what do you mean? Not following you here. What do you want to use the ref for? Something like this in the onSubmit? if (!inputRef?.current?.value.trim()) return
Keep in mind that we need to use I don't think state is an issue here..if anything I would change the useState to track input value, and derive inputLength from that. Did some digging on Vercel's AI chatbot and looks like they are passing input value state down to determine disabled button property value. References: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinmitch14 My bad! Didn't noticed you were trying to disable the send button on empty input, sorry.
With the last changes applied it looks good to me. Thanks!
Good to merge this @shadcn? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect. Thank you.
Co-authored-by: shadcn <[email protected]>
Co-authored-by: shadcn <[email protected]>
This PR fixes an issue where users are able to send empty chat messages.
Old:
New: